Mark local skill builds with an OCI descriptor annotation#5000
Merged
Conversation
5 tasks
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5000 +/- ##
==========================================
- Coverage 69.04% 68.99% -0.06%
==========================================
Files 553 554 +1
Lines 73016 73056 +40
==========================================
- Hits 50414 50404 -10
- Misses 19601 19649 +48
- Partials 3001 3003 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
JAORMX
approved these changes
Apr 22, 2026
Collaborator
JAORMX
left a comment
JAORMX
left a comment
There was a problem hiding this comment.
LGTM, do we want to remove the local build annotation when doing push?
Contributor
Author
I don't think we need that, If I understand correctly our |
Introduce a `dev.stacklok.toolhive.local-build` annotation that marks tags in the shared local OCI store as produced by a local `Build` rather than by an OCI pull. The marker lives on the descriptor entry in `index.json`, not inside the manifest blob, so the artifact digest is unchanged and nothing crosses the wire on push: oras-go resolves the push reference by digest, which returns a plain descriptor and strips descriptor annotations. Adds `tagAsLocalBuild` (Tag + annotation) and `isLocalBuild` helpers. Self-contained — no callers yet. Subsequent commits wire it into the skill service so `ListBuilds` can filter out artifacts pulled into the store by install or the content API only for caching. Made-with: Cursor
Skills pulled into the shared OCI store by install or the content API were tagged for caching and then surfaced by `ListBuilds`, making every previewed or installed remote skill look like a local build. Wire the local-build descriptor annotation into the skill service: - `Build` calls `tagAsLocalBuild` instead of `ociStore.Tag`, stamping the marker on the root-index descriptor entry in a single step. - `ListBuilds` iterates `ociStore.ListTags` and filters by `isLocalBuild` before running the usual metadata enrichment, so pulled tags are skipped. - `DeleteBuild` needs no change: removing the tag also drops its descriptor entry (and its annotation) from `index.json`. OCI pulls (install and content) continue to tag the local store as a cache, but `Registry.Pull` tags by digest — which resolves to a plain descriptor in oras-go — so no annotation lands on the tag and the cached artifact stays invisible to `ListBuilds`. Intent-documenting comments at each pull site explain why the marker is deliberately skipped there. Made-with: Cursor
Add regression tests that simulate the real `Pull` tagging side effect for both `GetContent` and `installFromOCI` and assert `ListBuilds` stays empty afterwards. The invariant under test is that `Registry.Pull` tags by digest — which yields a plain descriptor — so the `dev.stacklok.toolhive.local-build` annotation never lands on pulled tags and cached remote artifacts cannot silently masquerade as local builds. Made-with: Cursor
samuv
force-pushed
the
skill-local-build-2
branch
from
8749f22 to
0471ca5
Compare
github-actions
Bot
added
size/M
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GET /api/v1beta/skills/contentand OCI-based install both tag the shared local OCI store as a cache — thenListBuilds(GET /api/v1beta/skills/builds) iterates every tag and reports them as local builds, so every previewed or installed remote skill pollutes the "what have I built locally?" view.dev.stacklok.toolhive.local-build=trueannotation stamped on the root-index descriptor entry inindex.jsonwhenBuildtags, and read back byListBuildsas the filter.Registry.Pulltags by digest, and oras-go'soci.Store.Resolvereturns a plain descriptor for digest references (annotations stripped). So pulled blobs stay in the OCI store as a cache but their tag is invisible toListBuilds.Why not a
builds.jsonsidecar (the #4971 approach)?Single source of truth, no sidecar to drift, no stale-entry pruning on list, no atomic-write path to maintain. Most importantly: no migration heuristic — #4971 had a
looksLikePulledRefgrandfather rule that could silently hide a pre-existingmy-skill:v1.0.0tag, which is a real failure mode. With annotations, pre-feature builds simply don't show up until rebuilt. Honest gap beats silent misclassification. As a bonus, the same marker composes with locally-built container images / MCP server images in the future, not just skills.Type of change
ListBuilds/GET /api/v1beta/skills/builds)Test plan
go test ./pkg/skills/skillsvc/... -count=1)go build ./...)TestBuild_StampsLocalBuildAnnotationinspectsindex.jsondirectly to confirm the annotation lands on the descriptor entry afterBuildTestDeleteBuild/delete_removes_local-build_marker_from_index.jsonconfirms the annotation disappears with the tagTestGetContent/remote_pull_does_not_pollute_ListBuildsandTestInstallFromOCI_DoesNotLeakIntoListBuildssimulate the realPulltagging side-effect and assertListBuildsstays emptyChanges
pkg/skills/skillsvc/local_build_marker.goLocalBuildAnnotationconstant +tagAsLocalBuild/isLocalBuildhelpers. Usesstore.Target().Resolve/.Tagto round-trip the descriptor annotation throughindex.json.pkg/skills/skillsvc/build.goBuildcallstagAsLocalBuildinstead ofociStore.Tag.ListBuildsiteratesListTagsand filters byisLocalBuildbefore the usual metadata enrichment.DeleteBuildis unchanged; the annotation disappears with the descriptor entry.pkg/skills/skillsvc/content.goregistry.Pullsite: pulled tags are invisible toListBuildsbecausePulltags by digest, which yields a plain descriptor.pkg/skills/skillsvc/install_oci.gopkg/skills/skillsvc/build_test.goBuild/ListBuilds/DeleteBuildtests to usetagAsLocalBuildwhere they previously calledociStore.Tag. NewTestBuild_StampsLocalBuildAnnotation(readsindex.json) and updatedTestDeleteBuildcase. NewTestListBuildscases for pulled-tag hiding and the pull + build mix.pkg/skills/skillsvc/content_test.goremote pull does not pollute ListBuildscase.pkg/skills/skillsvc/install_oci_test.goTestInstallFromOCI_DoesNotLeakIntoListBuilds.Does this introduce a user-facing change?
Yes — the response of
GET /api/v1beta/skills/buildschanges.thv skill build(or equivalentPOST /api/v1beta/skills/build) appear. Pulled skills stay on disk as a cache (transparent speed-up for re-previews / re-installs) but are no longer surfaced as builds.Migration note
Unlike #4971, there is no automatic migration. Tags that exist in the OCI store from before this change lack the new annotation and therefore will not appear in
ListBuildsuntil they are rebuilt withthv skill build. This is a deliberate trade-off: it eliminates the heuristic (looksLikePulledRef) that could misclassify an already-pulled fully-qualified tag as a "grandfathered build", at the cost of a one-time visibility gap for pre-feature local builds.